Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix wasmer compile command for non-x86 target #3185

Merged
merged 1 commit into from
Sep 15, 2022

Conversation

flfymoss
Copy link
Contributor

@flfymoss flfymoss commented Sep 14, 2022

Description

Currently, wasmer compile command with specifying non-x86 (e.g. aarch64-apple-darwin) target doesn't work.
This change fixes the issue by not SSE2 to be force added for non-x86 targets.

(Codes are ported from

if target_triple.architecture == Architecture::X86_64 {
and
if target_triple.architecture == Architecture::X86_64 {
)

Possibly relates #2760 (comment)

Demo

Before

$ wasmer compile hello.wasm -o hello.wasmu
Compiler: cranelift
Target: aarch64-apple-darwin
✔ File compiled successfully to `hello.wasmu`.
$ wasmer run hello.wasmu
Hello
World!
$ wasmer compile --target aarch64-apple-darwin -o hello.wasmu hello.wasm
Compiler: cranelift
Target: aarch64-apple-darwin
✔ File compiled successfully to `hello.wasmu`.
$ wasmer run hello.wasmu 
error: failed to run `hello.wasmu`
│   1: failed to instantiate WASI module
╰─▶ 2: missing required CPU features: "EnumSet(SSE2)"

After

$ wasmer compile --target aarch64-apple-darwin -o hello.wasmu hello.wasm
Compiler: cranelift
Target: aarch64-apple-darwin
✔ File compiled successfully to `hello.wasmu`.
$ wasmer run hello.wasmu 
Hello
World!

Review

  • Add a short description of the change to the CHANGELOG.md file

@syrusakbary
Copy link
Member

On point! Thanks for the fix!

@flfymoss flfymoss force-pushed the fix-compile-command-for-arm branch from 5cdfe18 to 43e5407 Compare September 15, 2022 01:07
@flfymoss
Copy link
Contributor Author

flfymoss commented Sep 15, 2022

Ah sorry, I've fixed build issue (missing import) on cli-compiler.

@syrusakbary
Copy link
Member

bors r+

bors bot added a commit that referenced this pull request Sep 15, 2022
3185: Fix `wasmer compile` command for non-x86 target r=syrusakbary a=flfymoss

<!-- 
Prior to submitting a PR, review the CONTRIBUTING.md document for recommendations on how to test:
https://github.com/wasmerio/wasmer/blob/master/CONTRIBUTING.md#pull-requests

-->

# Description
<!-- 
Provide details regarding the change including motivation,
links to related issues, and the context of the PR.
-->

Currently, `wasmer compile` command with specifying non-x86 (e.g. `aarch64-apple-darwin`) target doesn't work.
This change fixes the issue by not SSE2 to be force added for non-x86 targets.


(Codes are ported from https://github.com/wasmerio/wasmer/blob/8a6c98702f1b8b63f5ab3d1b8c094959dd325afe/lib/cli/src/commands/create_obj.rs#L80 and https://github.com/wasmerio/wasmer/blob/d4a888abed6d877d46bf040a932e693ab39940b9/lib/cli/src/commands/create_exe.rs#L151 )

Possibly relates #2760 (comment)

## Demo

### Before
```sh
$ wasmer compile hello.wasm -o hello.wasmu
Compiler: cranelift
Target: aarch64-apple-darwin
✔ File compiled successfully to `hello.wasmu`.
$ wasmer run hello.wasmu
Hello
World!
$ wasmer compile --target aarch64-apple-darwin -o hello.wasmu hello.wasm
Compiler: cranelift
Target: aarch64-apple-darwin
✔ File compiled successfully to `hello.wasmu`.
$ wasmer run hello.wasmu 
error: failed to run `hello.wasmu`
│   1: failed to instantiate WASI module
╰─▶ 2: missing required CPU features: "EnumSet(SSE2)"
```

### After
```sh
$ wasmer compile --target aarch64-apple-darwin -o hello.wasmu hello.wasm
Compiler: cranelift
Target: aarch64-apple-darwin
✔ File compiled successfully to `hello.wasmu`.
$ wasmer run hello.wasmu 
Hello
World!
```

# Review

- [ ] Add a short description of the change to the CHANGELOG.md file


Co-authored-by: Go Murakami <[email protected]>
@bors
Copy link
Contributor

bors bot commented Sep 15, 2022

Build failed:

@flfymoss flfymoss force-pushed the fix-compile-command-for-arm branch from 43e5407 to 26848ef Compare September 15, 2022 04:13
@flfymoss
Copy link
Contributor Author

flfymoss commented Sep 15, 2022

I've also fixed code style. All other tests have passed, so I think it's all OK now.
Sorry for bothering you.

@syrusakbary
Copy link
Member

bors r+

@bors
Copy link
Contributor

bors bot commented Sep 15, 2022

Build succeeded:

@bors bors bot merged commit 3589094 into wasmerio:master Sep 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants